src/repositories/user.repository.ts (1)

166-174: 업데이트 검증 로직 추가 고려

현재 구현은 쿼리 실행 후 업데이트된 행의 수를 확인하지 않습니다. 존재하지 않는 사용자 ID가 전달되어도 오류 없이 성공으로 처리됩니다.

이것이 의도된 동작인지 확인해주세요. 만약 존재하지 않는 사용자에 대한 명시적인 에러 처리가 필요하다면 다음과 같이 개선할 수 있습니다:

 async unsubscribeNewsletter(id: number): Promise<void> {
   try {
-    const query = `UPDATE "users_user" SET newsletter_subscribed = false WHERE id = $1`;
-    await this.pool.query(query, [id]);
+    const query = `UPDATE "users_user" SET newsletter_subscribed = false WHERE id = $1 RETURNING id`;
+    const result = await this.pool.query(query, [id]);
+    if (result.rowCount === 0) {
+      throw new DBError('해당 사용자를 찾을 수 없습니다.');
+    }
   } catch (error) {
     logger.error('User Repo unsubscribeNewsletter Error : ', error);
     throw new DBError('뉴스레터 구독 해제 중 문제가 발생했습니다.');
   }
 }

src/routes/user.router.ts (1)

172-172: GET 메서드 사용의 보안 고려사항

이메일에서 직접 클릭하는 구독 해제 링크의 특성상 GET 메서드 사용이 일반적이고 사용자 편의성이 높습니다. 다만 다음의 보안 고려사항이 있습니다:

• 이메일 스캐너나 브라우저 프리페치가 의도치 않게 구독 해제를 트리거할 수 있음
• CSRF 보호가 없음
• 로그나 브라우저 히스토리에 이메일이 노출됨

보안을 강화하려면 다음 접근을 고려할 수 있습니다:

1. 이메일 대신 일회성 토큰을 쿼리 파라미터로 사용
2. 토큰 기반으로 사용자를 식별하고 만료 시간 설정
3. 구독 해제 확인 페이지를 표시하고 POST 요청으로 최종 처리

현재 구현도 일반적인 패턴이므로 비즈니스 요구사항에 따라 판단하시면 됩니다.

src/controllers/__test__/user.controller.test.ts (1)

409-447: 테스트 커버리지 개선 제안

기본적인 성공/실패 케이스는 잘 커버되어 있습니다. 다음의 추가 테스트 케이스를 고려해보세요:

it('서비스에서 에러 발생 시 nextFunction에 에러를 전달해야 한다', async () => {
  const email = 'test@example.com';
  mockRequest.query = { email };
  const error = new Error('Database error');
  mockUserService.unsubscribeNewsletter.mockRejectedValue(error);

  await userController.unsubscribeNewsletter(
    mockRequest as Request,
    mockResponse as Response,
    nextFunction
  );

  expect(nextFunction).toHaveBeenCalledWith(error);
  expect(mockResponse.redirect).not.toHaveBeenCalled();
});

it('이메일이 빈 문자열인 경우 BadRequestError를 던져야 한다', async () => {
  mockRequest.query = { email: '' };

  await userController.unsubscribeNewsletter(
    mockRequest as Request,
    mockResponse as Response,
    nextFunction
  );

  expect(nextFunction).toHaveBeenCalledWith(
    expect.objectContaining({
      message: '이메일이 필요합니다.',
    })
  );
});

src/controllers/user.controller.ts (1)

173-186: 이메일 검증 및 PII 로깅 개선 고려

현재 구현은 이메일 존재 여부만 확인하고 형식은 검증하지 않습니다. 또한 오류 로그에 이메일이 노출됩니다.

다음 개선 사항들을 고려해보세요:

1. 이메일 형식 검증 추가 (선택사항):

if (!email || typeof email !== 'string' || !email.includes('@')) {
  throw new BadRequestError('올바른 이메일 형식이 아닙니다.');
}

2. PII 로깅 최소화:

   } catch (error) {
-    logger.error(`뉴스레터 구독 해제 실패: [email: ${req.query.email}]`, error);
+    logger.error(`뉴스레터 구독 해제 실패`, error);
     next(error);
   }

현재 구현도 기본적인 요구사항을 충족하지만, 위 개선 사항들은 보안과 개인정보 보호를 강화할 수 있습니다.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/reposities/user.repository.ts:7-15
Timestamp: 2024-12-05T17:21:07.909Z
Learning: Function `findByUserEmail` in `src/reposities/user.repository.ts` is scheduled to be deleted and should be ignored in future code reviews.

src/services/user.service.ts (1)

178-181: 이전 리뷰에서 이미 지적된 문제입니다.

사용자를 찾지 못한 경우 조용히 실패하는 이 동작은 이전 리뷰에서 이미 논의되었습니다. 보안상 이메일 열거 공격을 방지하려는 의도라면, 해당 설계 결정을 명확히 하는 주석을 추가하는 것이 좋습니다.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro